-
Notifications
You must be signed in to change notification settings - Fork 96
Upgrade to Rails 5 #33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
98c4126 to
b6c484c
Compare
This sets the dependency to allow Rails 5. In upgrading to Rails 5 we require Ruby 2 and changes that are made here will not work on older versions of Rails. Because of that I'm bumping the version to 2.0.0.
`before_filter` is now `before_action`. I've changed this to remove the deprecation warning.
`render text:` is no longer a valid way to render just text content. The correct way is to use `render plain:`. `render text:` is deprecated.
Rails now requires kwargs in test requests. This sets `params`, `format`, and `headers` accordingly to fix the deprecation warning.
`render nothing:` is deprecated. If you want nothing rendered you should use `head :ok`.
In rails/rails@394b7be parameters set were changed to query strings to behave more like a real browser. In a browser the parameters `false` will never get sent as `false` and will always be `"false"`. Because of this change the test for passing `false` into this `Proc` was failing. We need to change it to check if the layout is not false rather than relying on the value in the parameters.
`assigns` was removed from Rails and using `instance_variable_get` is the new correct way to assign an instance variable in tests.
b6c484c to
675c431
Compare
Rack and Rails require Ruby 2.2.2 so we need to upgrade travis to use only Ruby 2.2.2 and set the minimum version in the gemspec. This means we'll have to split this peripheral gem into 2 releases. One for the 4 apps and one for the 5 apps because the changes required by Rails 5 won't work on older version.
675c431 to
20c50ad
Compare
|
👍 I hope that this gem keeps existing for Rails 5 |
| gem.license = 'MIT' | ||
|
|
||
| gem.add_dependency 'actionpack', '>= 4.0.0', '< 5.0' | ||
| gem.add_dependency 'actionpack', '~> 5.x' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there anything about this patch that precludes using this gem with Rails 4?
If not, I’d suggest changing this constraint to:
gem.add_dependency 'actionpack', '>= 4', '< 5.1'or
gem.add_dependency 'actionpack', '>= 4', '< 6'|
any plans to merge this in? |
| - 2.2.2 | ||
| gemfile: | ||
| - Gemfile | ||
| - gemfiles/Gemfile-edge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
edge will soon be 5.1, so we should have a 5.0 gemfile
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, edge is in allow-failures group
so a Gemfile-5-0-stable file should really be added
|
Somebody please, think of the devs |
|
Somebody please, think on the open source maintainers that also have their lives turbolinks/turbolinks#124 (comment) |
|
Manually merged into master in 309b429 |
This bumps the gem to Rails 5. Because of Rails and Rack requirements we need to run on Ruby 2.2.0. Additionally changes required by Rails 5 will not work on older versions of Rails so to deal with that I've done what we did with other apps, bump to the next major version and will be making a 1-0-stable version of the branch for changes needed pre Rails 5.